-
Notifications
You must be signed in to change notification settings - Fork 23
serialize http errors #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This looks great...taking me a while to take it all in, but I think it makes sense. One question generally: Are errors that bubble up always guaranteed to be wrapped in HttpError? Or should we generalize this more to the point where it's possible (perhaps with user-provided serializer classes) to serialize more error types (I'm thinking of ones in A thought on On your numbered discussion points:
|
This same thought did occur to me as well. I rejected it for my initial implementation because while it does feel like it resembles the JSON API style, I'm not sure that it really increases developer productivity. Under the current implementation, when I get an error response back from the server, to find the inner exception detail I just have to expand the inner property. Formatting errors like you are suggesting would make it more of a hassle: first you read the id, and then you have to manually find the matching record in the collection. This could be a real pain if the hierarchy is more than 2 oe 3 levels deep. Of course you can write a tool to do this for you, but I often find myself looking at raw responses when developing my API, and I think embedded errors would generally be preferable. I'm willing to change my mind if you feel strongly about it though. =)
I don't know if it's guaranteed that this happens. But the only ones I've personally seen are HttpError. I am in favor of the concept of pluggable formatters, though, which is basically what I meant by number 4 above. We can let the user inject constructor parameters that implement IErrorSerializer, IResourceSerializer, etc. This relates to the above point in that it would let someone write an IErrorSerializer that flattens the hierarchy rather than embedding inner errors.
The formatter should be agnostic to global configuration, I don't want to rely on statics there. I see a few possible solutions:
It does seem like error logging is not part of the role of the media type formatter. Maybe the action filter we discussed should be in charge of this. We could then generate the GUID in the filter, and add it as a key to the HttpError (which is just an IDictionary<string, object> after all). Then the ErrorSerializer could fetch the ID from the HttpError object itself, rather than generating its own (although we could have it still generate one if the ID key is missing from the HttpError, for example if the user never added our action filter). |
|
Okay, I got everything else merged, can you get the conflicts resolved on this one? We need to open an issue on error handling/filtering I guess, and continue the conversation there. Short answer: I think what I meant when I suggested relying on Okay that wasn't so short. Like I said, I'll open an issue for this so we can track it separately. |
b8b2c3f to
8288768
Compare
8288768 to
d4bc665
Compare
This implements serialization of HttpError objects, (which are are what WebApi passes when an action throws an exception), in a way that is compliant with the json-api spec. I may have over-engineered this a bit, but I wanted it to be testable.
The standard members of the error object are populated as follows:
I also wanted to include stack trace and a recursive inner exception. These data points don't really have equivalents in the json-api spec for errors, but I included them anyway as
stackTraceandinner.A few points for further discussion: